Cover notification and star mutations in GitHub guard - #12105
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟡 Changes recommended
Plain-text mutation responses bypass the new secrecy rules and remain publicly labeled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates GitHub guard coverage for notification and star mutations.
Changes:
- Corrects write/read-write classifications.
- Applies user-scoped DIFC labels.
- Adds classification and baseline regression tests.
File summaries
| File | Description |
|---|---|
src/tools.rs |
Updates operation inventories and tests. |
src/lib.rs |
Prioritizes account-scoped baseline inference. |
src/labels/tool_rules.rs |
Adds private user labels. |
src/labels/mod.rs |
Updates label tests. |
Review details
Suppressed comments (1)
guards/github-guard/rust-guard/src/labels/tool_rules.rs:890
- The star/unstar success payloads still bypass this new secrecy rule. Both upstream tools return a plain
NewToolResultText(...); after extraction,apply_singleton_fallback_if_neededclassifies that wrapper as server metadata and emitssecrecy: []without callingapply_tool_labels(lib.rs:361-390). As a result, response labeling remains public even though resource labeling is nowprivate:user. Apply tool-specific labels in the plaintext fallback and cover the actual wrapped success shape.
secrecy = private_user_label();
baseline_scope = Cow::Borrowed(scope_names::USER);
integrity = writer_integrity(scope_names::USER, ctx);
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| secrecy = private_user_label(); | ||
| baseline_scope = Cow::Borrowed(scope_names::USER); | ||
| integrity = writer_integrity(scope_names::USER, ctx); |
This comment has been minimized.
This comment has been minimized.
|
@copilot address review feedback |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
🔒 mcpg Read-Only Stress — default AWFSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: INCONCLUSIVE
No writes leaked. No FAIL conditions observed.
|
🔒 mcpg Read-Only Stress — docker-sbxSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: INCONCLUSIVE
No write leaked. No FAIL conditions observed. INCONCLUSIVE due to structural test-surface gaps (read-only backend + unauthenticated References: §33269560599
|
🔒 mcpg Read-Only Stress — gVisorSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: INCONCLUSIVE
|
The GitHub guard was missing or misclassifying several mutating upstream MCP tools, leaving notification and star operations without the intended write/read-write inventory coverage and DIFC labels.
DIFC labeling
private:userapproved:userRegression coverage